Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Emit doesn't update 2 way bound props #393

Merged
merged 5 commits into from
Feb 17, 2021

Conversation

nandi95
Copy link
Contributor

@nandi95 nandi95 commented Feb 16, 2021

No description provided.

@@ -82,4 +83,32 @@ describe('props', () => {
object: {}
})
})

it('should return the updated props on 2 way binding', async () => {
Copy link
Contributor Author

@nandi95 nandi95 Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is probably the wrong place for this test, please advise where it would be best.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is probably fine. Probably could re-think the test layout at some point.

@@ -48,7 +48,7 @@ function recordEvent(
events: Events,
vm: ComponentInternalInstance,
event: string,
args: Events[number]
args: unknown[]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I typed wrong in the previous PR, fixed here

src/emit.ts Outdated Show resolved Hide resolved
Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am somewhat surprised this works. We had some discussion around v-model testing, I think this is pretty good.

#279

@lmiller1990
Copy link
Member

We may revisit and revert this @nandi95. Based on #440 (comment)

In that case, you'd need to just change to use a vModel mounting option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants